Repository navigation
fix: harden start.sh - #7003
fix: harden start.sh#7003SeriousCoding789 wants to merge 9 commits into
Conversation
halibobo1205
left a comment
There was a problem hiding this comment.
Scope: this bundles four independent changes, and the commits already separate cleanly along them. Per CONTRIBUTING ("Create one PR for one issue"), could it be split?
fix(http): idle timeout at the connection limit (bb50d9ea80):HttpService.java,ConnectionLimitTest.javafix(config): fail startup on invalid DNS publish parameters (7dd470c42b,5dd60da703,a1b0c836cc):Args.java,ArgsTest.javarefactor(db): remove the deprecatedKhaosDatabase.getBranchoverload and an unused import (4a7acdcc28,e56b0d3e52)fix(script):start.shhardening (7b8ca8889b,6025d61078,57da1fd0f9,e68a2e5fc3,0479e69e55):start.sh,start.sh.simple,shell.md,README.md
Beyond the rule, it helps here: the open review items are almost all in start.sh, so 1–3 could land now; a start.sh regression could be reverted without reverting the HTTP fix; and the DNS fail-fast (a behaviour change from 4.8.2.3) and the new gpg requirement each get their own release note.
download() now prints a notice and returns non-zero; release files must be fetched and verified manually. It returns instead of exiting so that restart() still completes when rebuildManifest reaches it.
Downloads: - TLS certificates are verified and a failed request leaves no file. - Release jars must carry a valid GPG signature from the release key listed under "Integrity Check" in README.md. A jar that fails is not used: --upgrade keeps the old jar and --download the existing one. - The mainnet config comes from java-tron and the Nile config from nile-testnet. Bugs: - --stop looped forever, the node was restarted two or three times per run, and -c was passed again on every start. - A jar argument such as my.jar was ignored, and -j with a path lost the path before starting. - --net ignored an existing config file; --net and --release kept going after a failed download. - Linux heap sizing needed bc, and the macOS memory check failed with an expr error. - Manifest rebuilding used a wrong URL and never ran the plugin. It now runs and, by default, rewrites manifests of 128 MB or more. - The latest release was picked in name order, clone failures were ignored, $darwin was never set, and failures exited with status 0. Platforms: - start.sh reads the Java version and architecture of its JVM. JDK 8 keeps the CMS options and JDK 17 uses the ZGC options from start.sh.simple, which is removed. ARM64 downloads the aarch64 jars and skips manifest rebuilding. - -s stops the node and the open file limit is raised to 65535, as in start.sh.simple. The default JVM_MX, used on macOS, is 12g. README and shell.md describe the single script.
Downloads: - TLS certificates are verified and a failed request leaves no file. - Release jars must carry a valid GPG signature from the release key listed under "Integrity Check" in README.md. A jar that fails is not used: --upgrade keeps the old jar and --download the existing one. - The mainnet config comes from java-tron and the Nile config from nile-testnet. Bugs: - --stop looped forever, the node was restarted two or three times per run, and -c was passed again on every start. - A jar argument such as my.jar was ignored, and -j with a path lost the path before starting. - --net ignored an existing config file; --net and --release kept going after a failed download. - Linux heap sizing needed bc, and the macOS memory check failed with an expr error. - On macOS without JAVA_HOME the script derived JAVA_HOME=/usr from the /usr/bin/javac stub, and /usr/bin/java then ran itself forever; the JDK now comes from /usr/libexec/java_home. - Manifest rebuilding used a wrong URL and never ran the plugin. It now runs and, by default, rewrites manifests of 128 MB or more. - The latest release was picked in name order, clone failures were ignored, $darwin was never set, and failures exited with status 0. Platforms: - start.sh reads the Java version and architecture of its JVM. JDK 8 keeps the CMS options and JDK 17 uses the ZGC options from start.sh.simple, which is removed. ARM64 downloads the aarch64 jars and skips manifest rebuilding. - -s stops the node and the open file limit is raised to 65535, as in start.sh.simple. The default JVM_MX, used on macOS, is 12g. README and shell.md describe the single script.
Adds comments to the Java detection, the release download and signature check, the memory sizing, the manifest rebuild and the option handling of start.sh, describing what each branch does. JVM_MX drops from 12g to 9g; Linux derives the heap from the total memory, so the default applies to macOS only.
start.sh had several problems that predate this branch: - --stop matched every process whose command line contained the jar name and dropped any line containing "start", so it could kill other nodes and miss its own. The node id is now kept in <jar name>.pid; a directory with a start.log but no pid file was used by the old script and its node is still found by name. --stop also finds a node set up by --release or -cb from the parent directory, and says so when no node was started from the current one. - sh start.sh failed under dash; the script now reruns itself under bash. - an option without its value looped forever; needValue exits instead. - --disable-rewrite-manifest was only accepted as --disable-rewrite-manifes. - the first unknown option swallowed all following arguments; each unknown token is now passed on and parsing continues, with -- as an explicit end. - paths with spaces broke unquoted expansions; FullNode options are an array now. - --release downloaded straight onto FullNode.jar; it now uses a temporary file like --upgrade. - gc.log was archived before the node was stopped, and by --download. - a JDK that exists but cannot run went unnoticed, and a JVM that died right after the start was reported as running. - the manifest rebuild looked for the database next to the caller instead of the node directory, --net stored a relative config path, the gc log rotation deleted unrelated files, ulimit lowered a higher limit, and the rebuild message named logs/archive.log instead of logs/toolkit.log. shell.md documents the pid file, -- and the corrected option spelling.
0479e69 to
a33cdf4
Compare
Thanks for your suggestion. Created 3 new PRs as your suggested. This pr is kept for |
Address outstanding review feedback on start.sh: - --stop: match the running node by the full jar path, so a jar passed with -j as a path containing spaces is found instead of being missed and its pid file wrongly emptied. - --release: download config.conf to a temporary name and move it into place only on success, so a failed re-download no longer deletes the existing config and leaves the node on the jar-bundled defaults. - rebuild manifest: fetch and verify the tool before the node is stopped, and skip it up front when gpg is missing, so the node is not left down during the download and key-server lookup. - bound wget/curl transfers and the gpg key-server lookup with timeouts.
Fold the logging helper of the removed start.sh.simple into start.sh: info/warn/error print a timestamped, colored line to the terminal (ERROR on stderr) and append the same line, without color codes, to the start.log beside the JVM output. Lifecycle messages (stop, upgrade, download, signature check, rebuild, start) are then preserved when the script runs from cron, systemd or nohup instead of being lost with the terminal. Messages a function returns on stdout are left as plain echo.
What does this PR do?
start.shdownloads: TLS certificates are checked, and release jars must carry a valid GPG signature from the release key. The mainnet config is fetched from java-tron and the Nile config from nile-testnet.start.shon both x86_64 (JDK 8) and ARM64 (JDK 17): it picks the JVM options for the Java version it finds and the release jars for the JVM's architecture.start.sh.simpleis removed.start.shbugs:--stoplooping forever, the node being restarted several times per run,-cbeing passed repeatedly, a hang on macOS whenJAVA_HOMEis not set (the script derivedJAVA_HOME=/usr, so the/usr/bin/javastub ran itself forever; it now uses/usr/libexec/java_home),--stopkilling any process whose command line contains the jar name while missing a node whose arguments contain "start",sh start.shfailing under dash, an option without its value looping forever,--disable-rewrite-manifestonly being accepted misspelled, the first unknown option swallowing all following arguments, paths with spaces,--releasedownloading straight ontoFullNode.jar,gc.logbeing archived before the node was stopped, a JDK that exists but cannot run going unnoticed, and a JVM that dies right after the start being reported as running.start.sh, updateshell.md, and describe in the README "Integrity Check" section what the release key protects against.Why are these changes required?
start.shdownloaded withwget --no-check-certificateand checked the jar against asha256sum.txtthat releases do not publish, so a downloaded jar was never verified.start.shonly worked on x86_64 / JDK 8, since the JDK 17 JVM rejects its CMS options; ARM64 users needed a separate template.start.sh --stopfound the node withps -ef | grep -v start | grep FullNode.jar, so it could stop another node with the same jar name, or none at all. The node id is now kept in<jar name>.pid; a directory with astart.logbut no pid file was used by the previous script and its node is still found by name.This PR has been tested by:
start.shtest suite on macOS (bash 3.2), Ubuntu 22.04 (arm64 and amd64), Debian 11 and Rocky Linux 8 covers downloads with wget and curl, verification of the real release signatures,--release/--upgrade/--download/--net, failure paths, and the JVM command for every common option; the JDK 8 command is unchanged. With real JDKs,start.shstartsFullNode.jar --helpon macOS / JDK 8 and Linux amd64 / JDK 8, andFullNode-aarch64.jar --helpon Linux arm64 / JDK 17, both withJAVACMDpreset and with the script locating the JDK itself. The final version passes 196 checks on macOS, 112 with the real network (a downloadedFullNode-aarch64.jarmatches the GitHub digest; tampered, unsigned and unverifiable downloads are removed) and 39 in an Ubuntu 26.04 / JDK 17 container, includingsh start.shunder dash, two nodes with the same jar name in different directories, a node started by the previous script, and--stopfrom the parent of a--releasesetup.Follow up
None.
Extra details
start.shnow needsgpgand outbound HTTPS access to keys.openpgp.org or keyserver.ubuntu.com, from which the release key is fetched by its fingerprint. If neither key server can be reached, the downloaded jar is removed. This is documented inshell.md.start.sh. This protects against tampered release files, but not against a compromise of this repository, which hosts both the script and the release files.start.sh --stopis run in the directory the node was started from, or in the parent of a--release/-cbsetup; elsewhere it reports that no node was started there and exits 1. A start takes 3 seconds longer, the time used to confirm that the JVM is still running.--ends the script options; everything after it goes to FullNode unchanged.JVM_MXinstart.shis 9g. It applies on macOS; on Linux the heap is still sized from the machine's memory or-mem.start.shnow works as documented: when a LevelDB database exists andArchiveManifest.jaris missing, the verified plugin is downloaded and run before start. By default it only rewrites manifests of 128 MB or more (-m); it is skipped on ARM64 and can be turned off with-dr.KhaosDatabasechanges that were part of this PR are now in fix(http): time out idle connections at the connection limit #7020, fix(config): fail startup on invalid DNS publish parameters #7019 and refactor: remove a deprecated KhaosDatabase overload and unused imports #7021.